mi: Add support for multiple csi buffers#1000
Conversation
|
I believe there is a few more places we need to update with the CSI, but this is just an first pass of the idea. |
chorkin
force-pushed
the
user/chorkin/dual_csi_upstream_no_aem
branch
3 times, most recently
from
2ea32e4 to
7c402a2
Compare
|
@jk-ozlabs what do you think of this approach? |
OK, maybe mark as a draft then, so that we're not bugging @igaw too much 😄 |
Sounds good to me! The major design point here is whether we make the caller responsible for assigning CSIs. I think that's the right choice, but probably warrants some thought to confirm. |
chorkin
force-pushed
the
user/chorkin/dual_csi_upstream_no_aem
branch
4 times, most recently
from
66f414d to
ed09666
Compare
chorkin
force-pushed
the
user/chorkin/dual_csi_upstream_no_aem
branch
2 times, most recently
from
58fce7a to
e79ad02
Compare
|
I took a stab at it. Because the way some passthrough stuff was handled not everything uses the init functions. Let me know if you feel strongly and I can make more changes.
Sent from my Verizon, Samsung Galaxy smartphone
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Jeremy Kerr ***@***.***>
Sent: Friday, April 25, 2025 8:08:20 PM
To: linux-nvme/libnvme ***@***.***>
Cc: Chuck Horkin ***@***.***>; Author ***@***.***>
Subject: Re: [linux-nvme/libnvme] mi: Add support for multiple csi buffers (PR #1000)
@jk-ozlabs commented on this pull request.
________________________________
In src/nvme/mi.c<#1000 (comment)>:
@@ -929,6 +936,7 @@ static int __nvme_mi_admin_get_log(nvme_mi_ctrl_t ctrl,
ndw = (len >> 2) - 1;
nvme_mi_admin_init_req(&req, &req_hdr, ctrl->id, nvme_admin_get_log_page);
+ req_hdr.hdr.nmp |= (ctrl->ep->csi & 1);
Yes, the latter: we'd have a helper for the MI command inits (which are all currently open-coded), and do the nmp setup there.
That means we would just have the three locations for setting the CSI: each of the three command-type specific _req_init functions. If necessary, we could also add a helper (that those three helpers would themselves call) for just the common MI header too, but I'm not sure that's justified.
—
Reply to this email directly, view it on GitHub<#1000 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AWCQL3YRFQFFYRA4K5PCZTD23L2CJAVCNFSM6AAAAAB3UY5642VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOOJVG4YDINJRHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@jk-ozlabs , I think this is ready for full review at this point. |
jk-ozlabs
left a comment
jk-ozlabs
left a comment
There was a problem hiding this comment.
Looks good, those new helpers make things cleaner.
A couple of comments inline.
|
I see the two MI PRs getting ready to be merged. When both are ready, is there an order I have to pull them or they are completely independent? |
They are independent, but will generate merge conflicts, so I suggest we take whichever one is available first, and if they both are, let's do the AEM one first because it's much more changes. |
chorkin
force-pushed
the
user/chorkin/dual_csi_upstream_no_aem
branch
3 times, most recently
from
bc14996 to
e929fbd
Compare
These are used to send up to two concurrent requests over MI to the endpoint. Signed-off-by: Chuck Horkin <[email protected]>
igaw
force-pushed
the
user/chorkin/dual_csi_upstream_no_aem
branch
from
e929fbd to
acf75cf
Compare
|
rebased to resolve conflict in map file. |
|
Thanks! |
3 participants
These are used to send up to two concurrent requests over MI to the endpoint.